Fix: prevent unexpected navigation when destroying record from side panel#21391
Open
DeviSriSaiCharan wants to merge 15 commits into
Conversation
… people records page - This commit introduces isInSidePanel flag into the HeadlessCommandContextApi, passing it down from the command menu click event. DestroyRecordsCommand now check this flagl if the deletion originates from a side panel, the app simply closes the panel and keeps the user on their current page.
- Updated ObjectRecordOperation to include the specific destroyRecordIds in the browser event payload. - Introduced a global locallyDeletedRecordIdsAtom so the list of deleted records survives cell unmount/remounts during mouse hovers. - Updated RelationfromManyFieldDisplay to listen for destruction events, instantly log the destroyed IDs to the global state, and forcefully filter them out of the UI
…E database events
|
👋 Thanks for contributing to Twenty! Your PR has been set to draft while you work on it. Once you're done, mark it as Ready for review and our automated checks will run. Looking forward to your contribution! |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Propagates destroyed record IDs through ObjectRecordOperation browser events and uses them in relation field displays, plus adds context about whether a command is executed from the side panel to adjust post-destroy UI behavior.
Changes:
- Extend
ObjectRecordOperationto includedestroyedRecordId(s)for destroy operations and populate them at dispatch / SSE conversion points. - Filter
RelationFromManyFieldDisplayresults based on locally tracked destroyed IDs from browser events. - Add
isInSidePanelto the headless command context and close the side panel instead of navigating after single-record destroy.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/twenty-front/src/modules/sse-db-event/utils/turnSseObjectRecordEventToObjectRecordOperationBrowserEvent.ts | Adds destroyed record id(s) onto destroy operations derived from SSE events |
| packages/twenty-front/src/modules/object-record/types/ObjectRecordOperation.ts | Refines destroy operation union types to require destroyed id(s) |
| packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/RelationFromManyFieldDisplay.tsx | Tracks destroyed ids and filters displayed relation values accordingly |
| packages/twenty-front/src/modules/object-record/hooks/useIncrementalDestroyManyRecords.ts | Collects destroyed ids across batches and dispatches them in a browser event |
| packages/twenty-front/src/modules/object-record/hooks/useDestroyOneRecord.ts | Dispatches destroyed record id for destroy-one operations |
| packages/twenty-front/src/modules/object-record/hooks/useDestroyManyRecords.ts | Dispatches destroyed record ids for destroy-many operations |
| packages/twenty-front/src/modules/command-menu-item/hooks/useCommandMenuItemClick.ts | Passes isInSidePanel when mounting commands |
| packages/twenty-front/src/modules/command-menu-item/engine-command/utils/buildHeadlessCommandContextApi.ts | Threads isInSidePanel through the headless command context |
| packages/twenty-front/src/modules/command-menu-item/engine-command/types/HeadlessCommandContextApi.ts | Adds isInSidePanel to the headless context type |
| packages/twenty-front/src/modules/command-menu-item/engine-command/record/components/DestroyRecordsCommand.tsx | Closes side panel after single-record destroy when executed from side panel |
| packages/twenty-front/src/modules/command-menu-item/engine-command/hooks/useMountCommand.ts | Extends mount params to accept isInSidePanel |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
6 issues found across 11 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
… using atomFamily
Previously, the `locallyDeletedRecordIdsAtom` (used to instantly hide destroyed chips in `RelationFromManyFieldDisplay`) was defined as a single global array. This raised architectural concerns as it could create a small memory leak over long sessions (an ever-growing array) and caused all table cells across the application to share and iterate over the same list of destroyed IDs.
This commit refactors the state to use Jotai's `atomFamily`, keying the state dynamically by `${recordId}-${fieldName}`. This ensures the fallback state remains cleanly isolated per-cell while still surviving component unmounts/remounts during mouse hovers.
This commit resolves PR feedback regarding the event-driven fallback in RelationFromManyFieldDisplay: - Removed the empty string fallback (`?? ''`) for `objectMetadataItemId` to safely pass `undefined` and prevent the listener from subscribing to invalid keys. - Enforced strict equality (`===`) for the locally deleted IDs length check. - Wrapped state updates in `Array.from(new Set(...))` to guarantee ID uniqueness, ensuring fast lookups and preventing array bloat from duplicate destroy events.
…ve performance During bulk deletions, `useIncrementalDestroyManyRecords` was accumulating all destroyed IDs in memory and waiting until the entire operation finished before broadcasting the UI event. For massive deletions, this caused unnecessary memory bloat and delayed UI responsiveness. This commit removes the accumulation array and moves the event dispatch inside the `incrementalFetchAndMutate` loop. Destroy events are now broadcast progressively per-batch, bounding memory usage and ensuring the UI updates incrementally.
…oying-record-from-side-panel
🔍 Automated Pre-Review✅ No issues detected - This PR is ready for human review. 🧭 External PR Quality Review🟠 Needs triage for the following reason(s):
cc @prastoin Checks
Detailed findings (duplicate candidates, standards notes, summary) are in the workflow run logs. Automated pre-review — human approval still required. |
FelixMalfait
added
-PR: awaiting review
and removed
-PR: wip
-PR: awaiting author
labels
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: #21243
Issue
When a user is on a specific record's page (for example, looking at a Person) and opens a related record (like a Note) in the right-side panel, clicking "Permanently Delete" on that Note would abruptly redirect the user to the main "Notes" list. This breaks the user's workflow, as they typically want to remain on the parent Person page after deleting a sub-record.
Root Cause
Inside the
DestroyRecordsCommandandDeleteRecordsCommandcomponents, the application was programmed to unconditionally trigger anavigateAppredirect to the deleted object's index page upon successful deletion. The code did not account for whether the deletion was triggered from the main index page or from inside a contextual side panel.How we fixed it
I introduced a new
isInSidePanelflag into theHeadlessCommandContextApi.The command menu now detects if the delete action originated from inside a side panel and passes this flag down the execution chain. If
true, theDestroyRecordsCommandsimply closes the panel (closeSidePanelMenu()) and keeps the user exactly where they were.After that fix, I encountered another issue (UI not updating automatically)
Because the app now correctly kept the user on the Person page, a new bug surfaced: the "Note" chip inside the relation table did not disappear immediately. The user had to manually refresh the page to see the deletion.
This happened because:
useStatefallback was insufficient. The table component aggressively unmounts and remounts relation cells whenever a user hovers over them to display interactive controls, which would wipe the local React state clean and cause the "ghost chip" to reappear.##How I fixed that issue (and optimized it)
I built an event-driven fallback using global Jotai state to permanently hide the chips:
useDestroyManyRecordsanduseIncrementalDestroyManyRecordshooks to extract and broadcast only the confirmed destroyed IDs directly from the Apollo mutation response, preventing false-positive UI removals if the backend performed a partial delete.atomFamily): Instead of a leaky global array, we used Jotai'satomFamilyto dynamically generate a unique Noticeboard for every specific table cell (${recordId}-${fieldName}). This ensures the hidden-state survives mouse-hover unmounts while remaining cleanly isolated.RelationFromManyFieldDisplaycomponent was updated to defensively guard againstundefinedarray entries and strictly match deleted IDs only against valid foreign keys (ending in'Id'), preventing false-positive removals if a UUID happened to be pasted into a description field.Screen Recording
Screen.Recording.2026-06-10.at.11.04.43.AM.mov